Skip to content

fix(mobile): keep Android markdown icons aligned with text - #11079

Merged
shivamhwp merged 2 commits into
pingdotgg:mainfrom
SunkenInTime:t3code/fix-android-text-overlays
Sep 10, 2026
Merged

fix(mobile): keep Android markdown icons aligned with text#11079
shivamhwp merged 2 commits into
pingdotgg:mainfrom
SunkenInTime:t3code/fix-android-text-overlays

Conversation

@SunkenInTime

@SunkenInTime SunkenInTime commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Android renders inline Go/file icons on unrelated lines in Markdown lists, even with #10983 applied. Reproduced on upstream d29c56a5c4 with a Pixel 9 Pro / Android API 36 emulator and a synthetic version of the reported message.

Use the existing block renderer on Android so paragraph/list spacer line heights no longer share one native text layout. Keep the existing heading and paragraph spacing through one shared spacing helper. Add a nonbreaking space to keep each inline icon with its label when wrapping; the Android selection sanitizer removes that injected spacer together with the icon, while preserving authored whitespace. Native selection is now scoped to a paragraph or list item; the message copy button still copies the whole message. This is documented in the user guide.

The shared renderer covers messages and Markdown file previews. The change is Android-only, with no provider, wire-contract, or connection-mode changes.

Validation: clean Metro before/after bundles, repeated fixture loads, wrapped Go references, external-link icons, native text selection and file-reference navigation. Mobile typecheck and 136 focused Markdown/feed tests passed. Review follow-up verification includes all 35 Markdown tests, four Android API 36 Robolectric copy tests, and a rebuilt Android debug app. Targeted lint has no errors (two existing warnings in untouched code); formatting passed. The sanitizer change requires a new native Android build. The other reported activity-label overlaps did not reproduce on current upstream; this PR makes no claim to fix every timing-dependent overlap from the old Android release.

Before — September 10, 2026 After — September 10, 2026
Inline icons drift onto unrelated lines Icons stay aligned with file and link labels

Model: GPT-6. Harness: Codex.

Summary by CodeRabbit

  • Bug Fixes

    • Improved Android markdown rendering to keep file and link icons visually separated from their text.
    • Prevented inline images from shifting when displayed alongside paragraphs and list items.
    • Improved text selection behavior across Android markdown content.
    • Copied selections now exclude inline images and their spacing while preserving authored whitespace.
  • Documentation

    • Added guidance for selecting text in paragraphs or list items with a long press.
    • Documented how to copy an entire message using the copy button.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Sep 10, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 66e9443

Macroscope's review found this PR approvable — This is a localized Android markdown rendering fix that keeps inline icons aligned, preserves block spacing, and cleans injected selection whitespace. Production changes stay within the markdown component and are accompanied by focused TypeScript and Robolectric tests.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 10, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 9482d66b-0049-4b62-b5e7-3cb75602a287

📥 Commits

Reviewing files that changed from the base of the PR and between 8e520d3 and 66e9443.

📒 Files selected for processing (6)
  • apps/mobile/modules/t3-markdown-text/android/build.gradle
  • apps/mobile/modules/t3-markdown-text/android/src/main/java/expo/modules/t3markdowntext/T3MarkdownTextSelectionModule.kt
  • apps/mobile/modules/t3-markdown-text/android/src/test/java/expo/modules/t3markdowntext/MarkdownSelectionCopyTest.kt
  • apps/mobile/modules/t3-markdown-text/src/NativeMarkdownBlock.ios.tsx
  • apps/mobile/modules/t3-markdown-text/src/nativeMarkdownText.ts
  • apps/mobile/src/lib/nativeMarkdownText.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Android markdown rendering now uses native blocks for all chunks. Android block spacing is calculated by document structure, and copied selections remove inline images with their injected spacers. Composer documentation describes Android text selection and full-message copying.

Changes

Android Markdown selection

Layer / File(s) Summary
Native Android chunk rendering
apps/mobile/modules/t3-markdown-text/src/SelectableMarkdownText.ios.tsx, apps/mobile/modules/t3-markdown-text/src/NativeMarkdownSelectableText.ios.tsx, docs/user/composer.md
Android renders all chunks through NativeMarkdownBlock. Runs with file or link icons receive a leading non-breaking space. The composer documentation describes long-press selection and full-message copying.
Android block spacing
apps/mobile/modules/t3-markdown-text/src/nativeMarkdownText.ts, apps/mobile/modules/t3-markdown-text/src/NativeMarkdownBlock.ios.tsx, apps/mobile/src/lib/nativeMarkdownText.test.ts
The shared spacing helper returns distinct values for headings, post-heading blocks, body blocks, and the first block. Android applies these values to per-child views, with tests covering the resulting sequence.
Android selection copy sanitization
apps/mobile/modules/t3-markdown-text/android/src/main/java/expo/modules/t3markdowntext/T3MarkdownTextSelectionModule.kt, apps/mobile/modules/t3-markdown-text/android/src/test/java/expo/modules/t3markdowntext/MarkdownSelectionCopyTest.kt, apps/mobile/modules/t3-markdown-text/android/build.gradle
Copied selections omit inline images and their injected non-breaking spaces. Robolectric tests and Android unit-test configuration were added.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • pingdotgg/t3code#8779: Adds related Android markdown text-selection and copy-sanitization work in the same native module.

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 66e94

Android Markdown spacing and selection-copy behavior are covered for the changed cases, with no remaining merge-blocking risk identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 7 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the primary Android Markdown fix: keeping inline icons aligned with their text.
Description check ✅ Passed The description clearly explains the problem, implementation, scope, UI impact, validation, screenshots, and known limitations. It does not use the exact template headings or checklist, but it provide…
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 7 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@shivamhwp

Copy link
Copy Markdown
Collaborator

Note: GPT-6 on behalf of shivam (@shivamhwp).

The wrapping fix works: Go icons stay with their filenames, including wrapped and nested list items. File navigation, paragraph/list-item selection, and whole-message copy behave as expected.

One minor issue remains with native selection copy. The added nonbreaking space is copied along with the label. Selecting and copying the first list item produces \u00a0main.go:12 starts the server and registers routes. The icon placeholder is removed, but the injected U+00A0 remains.

Consider removing the injected spacing character alongside its inline image in the native copy sanitizer, while preserving spaces authored in the message. Whole-message copy is unaffected.

@SunkenInTime

Copy link
Copy Markdown
Contributor Author

@shivamhwp Fixed in 66e9443. The Android copy sanitizer now removes the one injected NBSP immediately following an inline image, including when selection begins after the image. Authored whitespace and literal object-replacement characters without image spans are preserved.

Added four focused Robolectric tests on API 36, all passing, and rebuilt the Android debug app successfully. The emulator still renders the wrapped Go references correctly. This native sanitizer change needs a new Android build; an older installed binary will keep the previous copy behavior.

@github-actions github-actions Bot added size:M 30-99 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Sep 10, 2026
@shivamhwp
shivamhwp merged commit 444fd8b into pingdotgg:main Sep 10, 2026
24 checks passed
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 10, 2026
## What's Changed
* fix(mobile): keep Android markdown icons aligned with text by @SunkenInTime in pingdotgg/t3code#11079
* Revert "fix(mobile): keep Android markdown icons aligned with text" by @juliusmarminge in pingdotgg/t3code#11098


**Full Changelog**: pingdotgg/t3code@v0.0.41-nightly.20260910.1486...v0.0.41-nightly.20260910.1507

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.41-nightly.20260910.1507
aorwall added a commit to aorwall/t3code that referenced this pull request Sep 11, 2026
Merges `upstream/main` at `02297e3db` into the fork, 35 commits from
base
`0f602b337`. Merge commit, not a rebase. Tracker entry:
`docs/fork/upstream-merge-log.md`, 2026-09-11.

`170` files landed against `166` in the upstream range; fork delta `756`
files.
The gap is six named files and reconciles:
`ThreadStatusIndicators.test.tsx` and
`sandboxControl.placement.test.tsx` landed as fork-test fixes amended
into the
merge, the three fork documents landed with it, and
`PreviewLocalServerCard.tsx` was re-deleted per the inventory's
deliberate-deletion list. `duplicate-adds.mjs` and
`resolution-check.mjs` are
clean — nothing landed as one side whole.

## What upstream shipped, and where it stands on Moatless

### Usable as-is

These run on the fork's backend with no further work.

- **A compact right-panel surface menu** (pingdotgg#11111) — the add-surface
launcher
goes from a card grid to keyboard-shortcut rows. This is where the
fork's
sandbox status badge lives, so the badge was re-stated on upstream's row
  rather than replayed; see the conflict notes below.
- **Multiple-linked-PR badges, simplified** (pingdotgg#11104, pingdotgg#11180, pingdotgg#11101) — a
thread
with several links now shows the total linked count coloured by
aggregate
status, instead of naming a primary and counting the extras. This is
**live on
  Moatless**: the backend serves `thread.pullRequests` and reports
`threadPullRequests`, so the badge resolves. It is also a behaviour
change a
user will see, and it is what broke the fork's own badge test — the only
thing
  that caught it.
- **Settled threads recede in the sidebar** (pingdotgg#11101) — a settled row
dims until
  hover or focus. Rides the settlement state Moatless already serves.
- **Environments in the command palette** (pingdotgg#10722) — searching now
returns
  environments beside threads and projects, with a subtitle.
- **A blue/orange diff palette** (pingdotgg#10671) — client-side theme only.
- **Question answers folded into tool activity** (pingdotgg#11014) — the chat
timeline
renders an answer to an agent's async question inside the tool call that
asked
  it, rather than as a separate turn. The client half
(`client-runtime/src/work-log/userInput.ts`,
`shared/src/toolActivity.ts`,
`MessagesTimeline.tsx`) works off data Moatless already sends. The
server half
  is in the third bucket.
- **Unpriced model activity is flagged** (pingdotgg#11021) — usage rows with
tokens and
no price read as unpriced instead of `$0.00`. Shared merge logic over
the
  `server.getUsageSummary` the backend serves.
- **Return to picture-in-picture when the right panel closes** (pingdotgg#11102)
and
**aligned floating-preview corners** (pingdotgg#10915) — both land in the hosted
preview surface and both were taken; the fork's framed-runtime condition
in
`ThreadPreviewMiniPlayer.tsx` still covers the case upstream's `framed`
check
  does not.
- **Collapse a tool call by clicking its expanded label** (pingdotgg#11017),
**provider
update text fitting inside sidebar notices** (pingdotgg#11034), **no seams in the
topbar scroll fade** (pingdotgg#10914), **centered PR unavailable states**
(pingdotgg#11110), **no
  sidebar PR link icon** (pingdotgg#11179).
- **Mobile** (pingdotgg#11128, pingdotgg#11127, pingdotgg#11114, pingdotgg#11115, pingdotgg#11118 / pingdotgg#11079 / reverted
in
pingdotgg#11098, pingdotgg#11113) — shared markdown renderer rename, composer transition
and
final-frame fixes, tablet close controls for files and terminal, and
playback
  preserved across fullscreen transitions.

### Unsupported in Moatless / needs implementation

- **The device hub** — the whole of pingdotgg#10677, pingdotgg#10854, pingdotgg#10855 and pingdotgg#10856:
iOS
simulators and Android emulators a person and an agent can share. Server
side
is `apps/server/src/device/` (`LocalDeviceHost.ts` drives the machine
the
  server runs on, `SshDeviceHost.ts` drives another over SSH,
`DeviceHubProxy.ts` fronts the video and accessibility streams) plus an
MCP
  toolkit at `apps/server/src/mcp/toolkits/device/` that gives the agent
  tap/type/screenshot. Client side is a `device` right-panel surface
  (`apps/web/src/components/device/`) and a Device hosts settings page.

Eight methods plus one stream, all newly declaring
`UnsupportedMethodError` in
  `packages/contracts/src/rpc.ts`: `device.configure`, `device.list`,
  `device.testHost`, `device.open`, `device.close`, `device.shutdown`,
`device.detail`, `device.action`, and the `subscribeDeviceState` push
stream.

  **Nothing gates the surface on a capability.** `ChatView.tsx` passes
`deviceAvailable={activeThreadRef !== null}`, so the launcher offers a
Device
row on every thread. `subscribeDeviceState` never resolves on Moatless,
so the
state stays empty, `onboardingCompleted` is false, and clicking the row
opens
`DeviceSetup` rather than the panel — whose first step, "Enable the
device
hub", calls `device.configure` and shows the refusal. A dead end a
person can
walk into. One additive `FEATURES.deviceHub` read on the two
`deviceAvailable`
props would drop the row instead; **this merge did not add it**, and it
is
recorded as the open work in `docs/fork/gaps.md` under _The device hub_.

Implementing it on the backend is a real question rather than a stub:
the hub
needs Xcode or the Android SDK on whatever host it drives, and its
stream is a
  second connection beside the RPC one.
- **Label and reviewer updates without redundant reloads** (pingdotgg#11117) —
optimistic
  cache writes in `client-runtime/src/state/pullRequests.ts` over
`pullRequests.update`. Rides the `pullRequests.*` group, which Moatless
does
not serve and which the `pullRequests` capability already keeps off, so
it
  changes nothing here until that group lands.
- **Emphasised primary PR actions** (pingdotgg#11105) and **save a PR body with
  Cmd/Ctrl+Enter** (pingdotgg#10660) — same surface, same condition.
- **Zed remote links accepting root paths and Windows servers** (pingdotgg#11044)
— builds
an SSH open target the Electron shell hands to a local editor. That path
needs
  a desktop shell, so it does not reach this fork's browser client;
`shell.openInEditor` remains unsupported. Not a fork target, listed for
  completeness.
- **Marketing** (pingdotgg#11146, pingdotgg#11145) — `apps/marketing` is upstream's own
site;
  inherited and inert here.

### Backend behavior to consider reproducing in Moatless

Server-side fixes upstream made to its own runtime. Moatless implements
the same
contract, so each is worth checking against its own implementation.

- **Claude launch args override the derived permission mode** (pingdotgg#11026,
`apps/server/src/provider/Layers/ClaudeAdapter.ts`). Upstream derives a
permission mode from the thread's settings and then appends the agent's
launch
args; an explicit `--permission-mode` in those args used to be
overridden by
the derived one instead of winning. If Moatless derives a permission
mode the
same way, a user who set the flag explicitly is being ignored in the
same
  place. Cheapest of the three to check.
- **Project identity resolved before legacy PR relinks** (pingdotgg#11045,
`apps/server/src/orchestration/Layers/OrchestrationEngine.ts`). A legacy
pull-request link is relinked to its thread on load; upstream now
resolves the
project's repository identity first, so a relink cannot bind a link to
the
  wrong project when two projects share a branch name. The fork fills
`thread.pullRequests` from a Task's GitHub bindings, so it has the same
ordering question: the binding has to know which project it belongs to
before
  it is attached.
- **Question answers in the published activity payload** (pingdotgg#11014,
`apps/server/src/orchestration/ActivityPayloadProjection.ts`). The
projection
now folds `projectQuestionToolInput` into the activity payload for both
`mcp_tool_call` and plain tool items, which is what lets a client render
the
  answer inside the tool call. Moatless does not report
`agentActivityPublishing`, so nothing reads this today — but if it ever
  publishes activity, this is the shape to publish.

## Conflicts and how they were resolved

11 conflicted files, resolved on the verdicts `preflight.mjs` printed.

Additive on both sides, both kept: `MessagesTimeline.tsx` (imports),
`client-runtime/src/rpc/client.ts` (the fork's four subscription tags
against
upstream's `subscribeDeviceState`), `rightPanelStore.ts` (the fork's
`sandbox`
kind against upstream's `device`), `rpc.ts` and `RpcAuthorization.ts`
(the
fork's thread-server / sandbox / subtasks methods and scopes against
upstream's
eight `device.*` ones), and `ChatView.tsx` (both right-panel arms, both
`onAdd*`
props at the inline and sheet call sites, and upstream's extended
`closePreviewPanel` under the fork's proactive preview-open effect).

`PreviewEmptyState.tsx` and `ThreadPreviewMiniPlayer.tsx` took
upstream's
`DiscoveryList` and `rounded-[inherit]` with the fork's sandbox-read
error line
and framed `hasPreviewSurface` condition re-stated on top.
`PreviewLocalServerCard.tsx` was a modify/delete conflict and was
re-deleted.
`pnpm-lock.yaml` auto-merged this time, so `--theirs` had nothing to do;
it was
reset to `upstream/main` and the fork edges re-derived with `vp i`.

Two findings worth reading:

**A `converged` delta can lose the line it was anchored to.** pingdotgg#11111
rebuilt the
add-surface menu from cards into rows, and the fork's sandbox badge in
`RightPanelTabs.tsx` (7 conflicts, the hard one) had no literal home
left. It
was re-stated on upstream's row — between the label and the `Kbd`,
additively,
no prop threaded and no upstream JSX re-indented — rather than replayed.
The
fork's placement test caught the other half: upstream's rows stopped
rendering
the action `description` at all, in its own Device action too, so the
test was
asserting on markup nobody emits. It now asserts the rendered label.

**A silent auto-merge changed behaviour with no marker, no type error
and no
resolution-check hit.** pingdotgg#11104 and pingdotgg#11180 changed what the multi-PR
badge counts
and hoisted `state` onto both badge shapes with a new `draft` colour.
Only the
fork's `ThreadStatusIndicators.test.tsx` failed, on `+1` against `+2`.
Fixture
gained the now-required `state`; the expectation was updated to
upstream's
semantics.

## Inventory and gaps

- New `pathPolicy` rows for the paths this merge decided without a
cached
  verdict: `right-panel-surfaces`, `client-runtime-rpc-client`,
`thread-status-indicators`, `fork-sandbox-components`.
`inventory-check.mjs`
  is clean.
- New gaps entry: _The device hub_, under **Methods the backend does not
dispatch**. It names the nine union entries holding it open, the missing
  `FEATURES.deviceHub` gate, and what closing it costs.
- Owned-concern sweep: 11 keyword hits, all false positives. Ten are
device-hub
files matching the `client-identity` concern on `host`/`proxy` — that
concern
  is about device *pairing* identity, not simulators — and the eleventh,
  `McpProviderSession.test.ts`, matched on `session`. No concern entry.

## Verification

`verify.mjs`: `duplicate-adds`, `tripwires`, `resolution-check`,
`unsupported-methods`, `fmt:check`, `lint` and `typecheck` pass.

`test` is red on **one** package, and it is not this merge:
`@t3tools/desktop`'s `scripts/browser-secret-native.test.mjs > bundled
libsecret
helper` cannot find `libsecret-1` in this sandbox's pkg-config path. A
missing
system package, not a code defect — 1 file failed of 102, and the
standing entry
is in `docs/fork/gaps.md` under _The desktop suite needs libsecret_.

One thing to know about reading that log: four packages did not finish
under
`vp run -r test` and were each re-run alone — mobile `157` files, `t3`
`315`,
web `389`, relay `30`, all passing. The truncated parallel pass reported
a
failure that does not exist, `shikiReviewHighlighter.test.ts >
initializes
source and snippet highlighting without a warmup`, which passes in the
alone run
and which neither side of this merge touches. The retry lines at the end
of the
log are the result; the parallel output above them is not.

Not reported green. The fork has no CI on pull requests
(`docs/fork/gaps.md`, _Nothing checks a pull request_), so this is the
whole of
the evidence.

---
Moatless task:
https://moatless.soaplabstest.com/tasks/211b4f8f-7de2-46c8-9ded-330d0cda4add
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants